Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix #4163, remove xdump and simplify dump #15707

Merged
merged 1 commit into from
Apr 26, 2016
Merged

fix #4163, remove xdump and simplify dump #15707

merged 1 commit into from
Apr 26, 2016

Conversation

JeffBezanson
Copy link
Sponsor Member

This replaces these functions with a single one that just shows everything inside an object.
Some examples:

julia> dump(("a",1+2im))
Tuple{ASCIIString,Complex{Int64}}
  1: ASCIIString
    data: Array(UInt8,(1,)) UInt8[0x61]
  2: Complex{Int64}
    re: Int64 1
    im: Int64 2

julia> dump(methods(sin).defs.func)
LambdaInfo
  code: Array(UInt8,(67,)) UInt8[0x10,0x88,0x88,0xcf,0x0e,0x10,0xb9,0x02,0x0a,0x66  …  0x1d,0x16,0x15,0x3b,0x0e,0x10,0x00,0x07,0x88,0x1d]
  slotnames: Array(Any,(2,))
    1: Symbol #self#
    2: Symbol a
  slottypes: Void nothing
  slotflags: Array(UInt8,(2,)) UInt8[0x00,0x00]
  gensymtypes: Int64 0
  rettype: Any
  sparam_syms: empty SimpleVector
  sparam_vals: empty SimpleVector
  tfunc: Void nothing
  name: Symbol sin
  roots: #undef
  specTypes: #undef
  unspecialized: #undef
  specializations: #undef
  module: Module Base
  def: LambdaInfo
# much longer but I'll cut it off here

julia> dump(StridedArray)
TypeConstructor
  parameters: SimpleVector
    1: TypeVar
      name: Symbol T
      lb: Union{}
      ub: Any
      bound: Bool false
    2: TypeVar
      name: Symbol N
      lb: Union{}
      ub: Any
      bound: Bool false
    3: TypeVar
      name: Symbol A
      lb: Union{}
      ub: DenseArray{T,N} <: AbstractArray{T,N}
      bound: Bool false
    4: TypeVar
      name: Symbol I
      lb: Union{}
      ub: Tuple{Vararg{Union{Base.AbstractCartesianIndex{N},Base.NoSlice,Colon,Int64,Range{Int64}}}} <: Any
      bound: Bool false
  body: Union{DenseArray{T,N},SubArray{T,N,A<:DenseArray{T,N},I<:Tuple{Vararg{Union{Base.AbstractCartesianIndex{N},Base.NoSlice,Colon,Int64,Range{Int64}}}},L}}

vtjnash
vtjnash reviewed Mar 31, 2016
View reviewed changes
base/show.jl
else
println(io, undef_ref_str)
end
for field in (isa(x,Tuple) ? (1:length(x)) : fieldnames(T))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably always be 1:nfields(T), since the indices are unique and always available

@JeffBezanson
Copy link
Sponsor Member Author

One remaining issue is that dump was intended to be overloaded, and several packages do so. However I don't think this is a good idea; dump should show concrete structure, and all other printing can be handled by show at different verbosity levels.

@tshort
Copy link
Contributor

tshort commented Apr 2, 2016

So, dump becomes what xdump was. That seems like a loss of functionality for viewing contents of tree-like structures. Consider the following with the existing dump function:

julia> d = Dict(:a => 1, :b => Dict(:x => Dict(:y => [2,3], :z => 3)))
Dict{Symbol,Any} with 2 entries:
  :a => 1
  :b => Dict(:x=>Dict{Symbol,Any}(:y=>[2,3],:z=>3))

julia> dump(d)
Dict{Symbol,Any} len 2
  a: Int64 1
  b: Dict{Symbol,Dict{Symbol,Any}} len 1
    x: Dict{Symbol,Any} len 2
      y: Array(Int64,(2,)) [2,3]
      z: Int64 3

julia> show(d)
Dict{Symbol,Any}(:a=>1,:b=>Dict(:x=>Dict{Symbol,Any}(:y=>[2,3],:z=>3)))

If I follow this right, that output of dump will go away, and it'll show the internals of the dict. The current output is much more helpful than that printed by show. Can we have another function that does what dump does now?

@JeffBezanson
Copy link
Sponsor Member Author

JeffBezanson commented Apr 2, 2016 via email

@ivirshup
Copy link
Contributor

ivirshup commented Apr 7, 2016

Can this be merged?

@ivirshup
Copy link
Contributor

Is anything not mentioned here stopping this from being merged?

If not, I'd be up for branching off this and fixing it up for a merge.

@JeffBezanson
Copy link
Sponsor Member Author

Basically just needs a rebase and a NEWS entry. I'll do that and merge.

@JeffBezanson JeffBezanson merged commit 005bb21 into master Apr 26, 2016
tkelman
tkelman reviewed Apr 26, 2016
View reviewed changes
base/docs/helpdb/Base.jl
@@ -1692,7 +1692,7 @@ split
"""
dump(x)

Show all user-visible structure of a value.
Show every part of the representation of a value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please run genstdlib and commit updates at the same time

@tkelman
Copy link
Contributor

tkelman commented Apr 26, 2016

several places in docs need fixing

@tkelman tkelman deleted the jb/dump branch April 26, 2016 03:25
@tkelman tkelman added the needs docs Documentation for this change is required label Apr 26, 2016
@ivirshup
Copy link
Contributor

ivirshup commented Apr 26, 2016 •
edited
Loading

What else needs fixes past types.rst?

@tkelman
Copy link
Contributor

tkelman commented Apr 26, 2016 

base/show.jl:1040:                #    xdump(fn, io, fieldtype(x,idx), n - 1, string(indent, "  "))
contrib/BBEditTextWrangler-julia.plist:1219:            <string>xdump</string>
doc/devdocs/types.rst:93:   julia> xdump(Array)
doc/devdocs/types.rst:107:   julia> xdump(T)
doc/devdocs/types.rst:136:   xdump(tv)
doc/devdocs/types.rst:172:   julia> xdump(p1[1].parameters[1])
doc/devdocs/types.rst:179:   julia> xdump(p3[1].parameters[1])
doc/devdocs/types.rst:313:   julia> xdump(Array.name)
doc/stdlib/io-network.rst:516:.. function:: xdump(x)

ivirshup added a commit to ivirshup/julia that referenced this pull request Apr 26, 2016
@ivirshup
Fixed up docs for changed dump function.
29cf874 
For JuliaLang#15707. In addition:

* Some commented out code referring to `xdump` was removed from `base/show.jl`.
* `doc/stdlib/arrays.rst` was updated from building docs.
@ivirshup ivirshup mentioned this pull request Apr 26, 2016
Merged
@tkelman tkelman removed the needs docs Documentation for this change is required label Apr 26, 2016
ivirshup added a commit to ivirshup/julia that referenced this pull request Apr 26, 2016
@ivirshup
Fixed up docs for changed dump function.
5009671 
For JuliaLang#15707. In addition:

* Some commented out code referring to `xdump` was removed from `base/show.jl`.
* `doc/stdlib/arrays.rst` was updated from building docs.
@omus omus mentioned this pull request Jul 13, 2016
Closed
@yuyichao yuyichao mentioned this pull request Apr 30, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JeffBezanson @tshort @ivirshup @tkelman @vtjnash